Conversation
|
I have passed the iOS/macOS full platform and full architecture compilation, we need to confirm the Android platform, and can be merged after the test is correct |
|
I tried to compile on windows, android, macOS, and android, everything seems to be ok. |
|
Great!! yes we should check if our previously applied android patches are still relevant (required). |
|
Looks like #11 is still required for NPE handling. IIRC, the patch I pulled that from was fairly recent (Oct 2021ish) so probably isn't in m93. |
I believe they are included in this branch (as this is a diff from chrome M93 to main). @davidliu could you confirm this is working as expected for Android? Then we can merge & release. |
|
Yeah, looked at the branch's code, the appropriate NPE handling is missing (it reverted back to the old non NPE handling code). |
ah I understand now. @cloudwebrtc could you re-apply #11? Are other patches kept in this PR? |
|
@cloudwebrtc @davidliu The problem seems to be more complicated than expected, Some patches are indeed covered I haven't checked all the patches. |
|
This way of maintaining repo does not seem to be perfect. Once the new upstream is merged, the patches we have done will be overwritten without prompting. |
|
#11 isn't covered. It's changed since M92, but it still doesn't cover the case where the primary encoder is null. |
|
Checked out the new commit, LGTM. |
That's a good point.. the other two options are
Thinking out loud.. perhaps 1. is better than our current approach.. it'll make it clear what we have changed. |
|
@hiroshihorie #7 This PR also seems to be covered. We need to confirm whether it affects iOS |
No description provided.